Skip to content

Conversation

vtardy-st
Copy link
Contributor

some header files and system files of the Link Layer in zephyr are not aligned with the header file and system files of the Link Layer in STM32Cube_FW_WBA Release 1.6.0

Copy link
Member

@erwango erwango left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general, it is unclear why this PR is needed as #280 already aligned lib on v1.6.0. Am I mixing something ?

Comment on lines 9 to 16
* This Synopsys DWC Bluetooth Low Energy Combo Link Layer/MAC software and
* associated documentation ( hereinafter the "Software") is an unsupported
* proprietary work of Synopsys, Inc. unless otherwise expressly agreed to in
* writing between Synopsys and you. The Software IS NOT an item of Licensed
* Software or a Licensed Product under any End User Software License Agreement
* or Agreement for Licensed Products with Synopsys or any supplement thereto.
* Synopsys is a registered trademark of Synopsys, Inc. Other names included in
* the SOFTWARE may be the trademarks of their respective owners.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For Zephyr licence compatibility purposes, files in this module where updated to the latest License which doesn't contain these lines. Please keep it as is. Apply to all files
See #262.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

License comments in Link Layer header files are changed as required

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Files should reflect Cube package library and changes should be documented and justified, ideally one by commit and
in this particular case, I think a deeper discussion is required.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LOG Module is reverted. I will insert a new specific PR for this feature

@vtardy-st vtardy-st force-pushed the stm32wbax_ble_update branch from 12c1ca9 to 4c5c07f Compare August 25, 2025 14:02
@vtardy-st
Copy link
Contributor Author

This PR is due to the fact that in previous PR#280 (aligned lib on v1.6.0), the header files of the Link Layer have not been updated correctly.

@vtardy-st vtardy-st requested a review from erwango August 25, 2025 14:07
Copy link
Collaborator

@asm5878 asm5878 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we want to be strict with Zephyr guidelines we should modify some modify not required changes, tabs, spaces.
If we want to maintain exactly the same cube code we can keep the changes.
For me LGTM, but I leave @erwango to decide.

@erwango
Copy link
Member

erwango commented Aug 26, 2025

@Chastillon fyi

@erwango
Copy link
Member

erwango commented Aug 26, 2025

If we want to be strict with Zephyr guidelines we should modify some modify not required changes, tabs, spaces.
If we want to maintain exactly the same cube code we can keep the changes.

Sure, we should keep the files as close as Cube, with the exception of line endings (use dos2unix) and trailing white spaces. These 2 changes are enforced when files are updated with the dedicated script.

Copy link
Member

@erwango erwango left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To ease the work for the next update and fix the trailing spaces issues I suggest that you run the update script (which should already have been the case at last update).

@vtardy-st vtardy-st force-pushed the stm32wbax_ble_update branch from 4c5c07f to e6daffc Compare August 27, 2025 06:58
@vtardy-st vtardy-st requested review from erwango and asm5878 August 27, 2025 07:49
@@ -64,7 +64,6 @@ else()
set(BLE_LIB_TYPE "BLE_LIB_BASIC")
endif()


Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not strictly need, I would prefer to minimize changes in this PR.

@@ -16,7 +15,6 @@
*
******************************************************************************
*/
/* USER CODE END Header */

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not strictly need, I would prefer to minimize changes in this PR.
This is valid for all changes in this file.

Copy link

@msmttchr msmttchr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vtardy-st, thanks a lot for this PR. I suggested some changes to make this PR minimal, and I have asked few questions regarding the actual usage of some files (It is requested in ST strategy in Zephyr to avoid adding files not used).
Is this PR needed to add 15.4 support ? If yes, I do not see 15.4 libraries added in the list of blob files (zephyr/module.yml), if not, ignore this comment.

@@ -1,4 +1,3 @@
/* USER CODE BEGIN Header */

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not strictly need, I would prefer to minimize changes in this PR.
This is valid for all changes in this file.

@@ -0,0 +1,229 @@
/*$Id: //dwh/bluetooth/DWC_ble154combo/firmware/rel/2.00a-lca01/firmware/public_inc/dtm.h#1 $*/

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this file needed ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file is a header file for Synopsys zigbee phy testing APIs, I assume that this one is not needed

@@ -0,0 +1,226 @@
/*$Id: //dwh/bluetooth/DWC_ble154combo/firmware/rel/2.00a-lca01/firmware/public_inc/ll_error.h#1 $*/

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this file needed ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file contains error defines across BLE FW LL, I will keep it

@@ -0,0 +1,556 @@
/*$Id: //dwh/bluetooth/DWC_ble154combo/firmware/rel/2.00a-lca01/firmware/public_inc/platform.h#1 $*/

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this file needed ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file would be used when 15.4 will be supported. I can remove it for this PR and add it for the next PR dedicated of driver 802.15.4....or keep it for this PR
What is the best way ?

@@ -0,0 +1,1543 @@
/*$Id: //dwh/bluetooth/DWC_ble154combo/firmware/rel/2.00a-lca01/firmware/public_inc/ral.h#1 $*/

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this file needed ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file would be used when 15.4 will be supported. I can remove it for this PR and add it for the next PR dedicated of driver 802.15.4....or keep it for this PR
What is the best way ?

@@ -0,0 +1,54 @@
/*$Id: //dwh/bluetooth/DWC_ble154combo/firmware/rel/2.00a-lca01/firmware/public_inc/rfd_dev_config.h#1 $*/

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this file needed ?

@@ -0,0 +1,25 @@
/**

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this file needed ?

@@ -101,7 +101,7 @@ static void ll_sys_dependencies_init(void)

/* Deep sleep feature initialization */
dp_slp_status = ll_sys_dp_slp_init();
ll_sys_assert(dp_slp_status == LL_SYS_OK);
ll_sys_assert(dp_slp_status == LL_SYS_OK);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not strictly need, I would prefer to minimize changes in this PR.

@@ -48,15 +48,21 @@
+ "DWC_ble154combo.h",
"Middlewares/ST/STM32_WPAN/link_layer/ll_cmd_lib/inc/bsp.h",
"Middlewares/ST/STM32_WPAN/link_layer/ll_cmd_lib/inc/common_types.h",
"Middlewares/ST/STM32_WPAN/link_layer/ll_cmd_lib/inc/dtm.h",

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are these changes really needed ?

@erwango
Copy link
Member

erwango commented Aug 27, 2025

I suggested some changes to make this PR minimal,

Actually, PR should contain required changes to have files on par with the Cube release they're extracted from (with the exception of trailing white space removal) here: https://github.com/STMicroelectronics/STM32CubeWBA/tree/v1.6.0/Middlewares/ST.
If we take less than this delta will increase for next tag and changes will be harder to integrate.
Using the update script and starting from a sane base, next update should be simple.

Update Link Layer header files and LE porting files to be
fully aligned on STM32Cube_FW_WBA Release 1.6.0.

Signed-off-by: Vincent Tardy <[email protected]>
@vtardy-st vtardy-st force-pushed the stm32wbax_ble_update branch from e6daffc to 384b73d Compare August 29, 2025 12:23
@vtardy-st
Copy link
Contributor Author

An update has been done in order to be fully aligned with the CubeMx release 1.6.0.
These update generates minor formal changes but I have preferred to be fully aligned with the official Cube files

@vtardy-st vtardy-st requested a review from msmttchr August 29, 2025 12:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants